feat(packages/config,packages/core): add root section type for flat sidebar grouping#97
Open
zrosenbauer wants to merge 11 commits intomainfrom
Open
feat(packages/config,packages/core): add root section type for flat sidebar grouping#97zrosenbauer wants to merge 11 commits intomainfrom
zrosenbauer wants to merge 11 commits intomainfrom
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis pull request adds an optional 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/core/src/sync/index.ts (1)
379-393: 🧹 Nitpick | 🔵 TrivialUpdate JSDoc to reflect root section handling.
The function now collects scope paths for both
standaloneandrootsections, but the JSDoc only mentionsstandalone: true. Consider updating the documentation to reflect the expanded behavior.📝 Suggested JSDoc update
/** - * Collect standalone sidebar scope paths from resolved entries. + * Collect standalone and root sidebar scope paths from resolved entries. * * Returns an array of link paths (e.g. `["/packages", "/contributing"]`) - * for sections that have `standalone: true`. These paths are written to + * for sections that have `standalone: true` or `root: true`. These paths are written to * `.generated/scopes.json` and consumed at runtime by the custom Sidebar * component to isolate standalone sections into their own sidebar scope. * * `@private` * `@param` entries - Top-level resolved entries - * `@returns` Array of standalone scope path strings + * `@returns` Array of standalone and root scope path strings */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/sync/index.ts` around lines 379 - 393, Update the JSDoc for function collectStandaloneScopePaths to state that it returns link paths for sections marked standalone OR root (not just standalone), and clarify that it collects paths from resolved top-level entries (parameter entries: readonly ResolvedEntry[]) which have either standalone or root set and a link present; adjust the description and example return comment to mention both standalone and root handling so docs match the implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.joggr/.gg/state.json:
- Around line 1-3: Add the `.joggr/` directory to version control ignore and
stop tracking the local state file `.joggr/.gg/state.json`: update `.gitignore`
to include a line with `.joggr/`, then remove the already committed `state.json`
from the repo index (e.g., git rm --cached) so it remains local only; ensure no
other project files depend on `.joggr/.gg/state.json` before removing it and
commit the updated `.gitignore`.
In @.superpowers/brainstorm/83668-1775853195/content/component-preview.html:
- Around line 1-110: The file is an HTML fragment (starts with <h2>New
Components Preview</h2>) and fails linting; wrap the fragment in a complete HTML
document by adding a doctype and html root, a head with meta charset and
viewport and a <title>, and move the existing fragment into a <body> (preserve
existing classes like "mockup-header"/"mockup-body" and sections such as the
Steps/ParamField content). Ensure proper closing tags for html and body so
component-preview.html passes lint and preview portability.
In @.superpowers/brainstorm/83668-1775853195/content/field-design.html:
- Around line 214-215: The expandable trigger buttons (the element with class
zp-field-group__trigger that calls toggleExpandable(this) and the other similar
trigger at lines referenced) lack an explicit type, which defaults to submit in
forms; update both <button> elements that invoke toggleExpandable to include
type="button" so they do not submit enclosing forms accidentally—ensure the
change is applied to the button using class zp-field-group__trigger and any
other matching trigger button in the same component.
- Around line 161-163: The disclosure currently only toggles the CSS class in
function toggleExpandable; update it to also set aria-expanded on the trigger
and ensure aria-controls exists on the trigger pointing to the expandable
region: inside toggleExpandable (and the other identical handlers), find the
trigger element (the clicked button passed as el or its closest trigger) and
update trigger.setAttribute('aria-expanded', isOpen) when toggling, and ensure
the expandable content has an id (generate a stable unique id if missing) and
set trigger.setAttribute('aria-controls', contentId); keep the existing class
toggle logic but keep aria-expanded in sync on every open/close.
- Line 315: The anchor using href="#" for the "UserConfig" link causes page jump
and bad history; replace it with a real URL to the UserConfig documentation if
navigation is intended, or convert the element to a non-navigating control
(e.g., a <span> or <button> with appropriate role/aria attributes and an onclick
handler) if it should open a modal/tool-tip or trigger JS, and remove the
href="#" to prevent history navigation; update the element referenced as
"UserConfig" accordingly and ensure accessibility attributes are present when
changing to a non-link control.
In @.superpowers/brainstorm/83668-1775853195/content/waiting.html:
- Around line 1-3: This fragment currently lacks a document wrapper and violates
doctype-first; either wrap the existing root <div> (the element with
style="display:flex;..." containing the <p class="subtitle">Building Steps and
ParamField components...</p>) in a valid HTML document (prepend <!DOCTYPE html>,
add <html><head> with at least a <meta charset="utf-8"> and optional <title>,
then place the existing <div> inside <body>) or, if it is intentionally a
partial, keep it as-is but mark it as a fragment by excluding this file from
HTMLHint (or move to a fragment-only directory) so the linter doesn't enforce
doctype-first.
In @.superpowers/brainstorm/83668-1775853195/state/server-stopped:
- Line 1: Remove the ephemeral development state file (server-stopped in the
.superpowers brainstorm state directory) from the commit and prevent it from
being re-added: remove it from the index (unstage/ git rm --cached), add the
appropriate pattern to .gitignore to exclude the .superpowers/brainstorm state
directory or its server-stopped file, and create a follow-up commit that only
contains the deletion and .gitignore update so runtime artifacts are not tracked
going forward.
In @.superpowers/brainstorm/83668-1775853195/state/server.pid:
- Line 1: Remove the runtime PID artifact from the commit: delete the server.pid
file and the server-stopped file from the commit history/staging so they are not
tracked (unstage/remove them from the index), and add the .superpowers/
directory to .gitignore to prevent future commits; ensure .gitignore entry
covers .superpowers/ and verify the files are no longer tracked (e.g., git
status shows them absent from staged changes) before finalizing the PR.
---
Outside diff comments:
In `@packages/core/src/sync/index.ts`:
- Around line 379-393: Update the JSDoc for function collectStandaloneScopePaths
to state that it returns link paths for sections marked standalone OR root (not
just standalone), and clarify that it collects paths from resolved top-level
entries (parameter entries: readonly ResolvedEntry[]) which have either
standalone or root set and a link present; adjust the description and example
return comment to mention both standalone and root handling so docs match the
implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d6cbf49c-719e-4c2a-a64a-67bc4bf3af08
📒 Files selected for processing (17)
.gitignore.joggr/.gg/state.json.superpowers/brainstorm/83668-1775853195/content/component-preview-v2.html.superpowers/brainstorm/83668-1775853195/content/component-preview.html.superpowers/brainstorm/83668-1775853195/content/field-design.html.superpowers/brainstorm/83668-1775853195/content/waiting.html.superpowers/brainstorm/83668-1775853195/state/server-stopped.superpowers/brainstorm/83668-1775853195/state/server.pidpackages/config/src/schema.tspackages/config/src/types.tspackages/core/src/sync/index.tspackages/core/src/sync/resolve/index.tspackages/core/src/sync/sidebar/index.tspackages/core/src/sync/sidebar/meta.test.tspackages/core/src/sync/sidebar/meta.tspackages/core/src/sync/sidebar/nav.test.tspackages/core/src/sync/types.ts
.joggr/.gg/state.json
Outdated
Comment on lines
+1
to
+3
| { | ||
| "active_project": null | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if .joggr/.gg/ is already in .gitignore and look for other .joggr state files
echo "=== Checking .gitignore for .joggr patterns ==="
if [ -f .gitignore ]; then
rg -n '\.joggr' .gitignore || echo "No .joggr patterns found in .gitignore"
else
echo ".gitignore not found"
fi
echo -e "\n=== Looking for other .joggr files in the repository ==="
fd -H -t f . .joggr 2>/dev/null || echo "No other .joggr files found"Repository: joggrdocs/zpress
Length of output: 226
🏁 Script executed:
find . -maxdepth 2 -name "*.md" -o -name "README*" -o -name "*.txt" | head -20Repository: joggrdocs/zpress
Length of output: 191
🏁 Script executed:
# Search for any references to .joggr or joggr in documentation or config files
rg -i "joggr" --type md --type txt --type json -g "!.joggr" -B 2 -A 2 | head -50Repository: joggrdocs/zpress
Length of output: 2798
🏁 Script executed:
# Check what files are in .joggr directory
ls -la .joggr/ 2>/dev/null || echo "No .joggr directory at root"
ls -la .joggr/.gg/ 2>/dev/null || echo "No .joggr/.gg directory"
cat .joggr/.gg/state.json 2>/dev/null || echo "Cannot read state.json"Repository: joggrdocs/zpress
Length of output: 417
🏁 Script executed:
rg -i "\.joggr|\.gg" --type ts --type js --type json -g "!.joggr" -B 2 -A 2Repository: joggrdocs/zpress
Length of output: 1175
🏁 Script executed:
# Also check if there's any documentation about tool state in README or CONTRIBUTING
rg -i "state|joggr" README.md CONTRIBUTING.md 2>/dev/null | head -30Repository: joggrdocs/zpress
Length of output: 411
🏁 Script executed:
# Check git status to see how this file was added
git log --oneline -10
git show --name-status | head -50Repository: joggrdocs/zpress
Length of output: 1601
Add .joggr/ to .gitignore to exclude local tool state files.
The .joggr/.gg/state.json file contains local tool state and is not referenced anywhere in the codebase. State files in hidden directories should not be version-controlled to prevent merge conflicts and repository clutter when multiple developers work locally. Add .joggr/ to .gitignore unless there's a specific reason this directory should be committed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.joggr/.gg/state.json around lines 1 - 3, Add the `.joggr/` directory to
version control ignore and stop tracking the local state file
`.joggr/.gg/state.json`: update `.gitignore` to include a line with `.joggr/`,
then remove the already committed `state.json` from the repo index (e.g., git rm
--cached) so it remains local only; ensure no other project files depend on
`.joggr/.gg/state.json` before removing it and commit the updated `.gitignore`.
Comment on lines
+1
to
+110
| <h2>New Components Preview</h2> | ||
| <p class="subtitle">Steps and ParamField — visual mockups of what was built</p> | ||
|
|
||
| <div class="section"> | ||
| <h3>Steps / Step</h3> | ||
| <p>Vertical timeline stepper with numbered circles and connecting lines.</p> | ||
|
|
||
| <div class="mockup"> | ||
| <div class="mockup-header">Steps — Basic Usage</div> | ||
| <div class="mockup-body" style="padding: 24px; font-family: -apple-system, system-ui, sans-serif; background: #fff;"> | ||
| <!-- Step 1 --> | ||
| <div style="display: flex; gap: 16px;"> | ||
| <div style="display: flex; flex-direction: column; align-items: center; flex-shrink: 0;"> | ||
| <span style="display: flex; align-items: center; justify-content: center; width: 24px; height: 24px; border-radius: 50%; background: #6466f1; color: #fff; font-size: 12px; font-weight: 600;">1</span> | ||
| <div style="width: 2px; flex: 1; background: #e5e7eb; min-height: 16px;"></div> | ||
| </div> | ||
| <div style="padding-bottom: 24px; flex: 1;"> | ||
| <h3 style="margin: 0; font-size: 16px; font-weight: 600; line-height: 24px;">Install zpress</h3> | ||
| <div style="margin-top: 8px; font-size: 14px; color: #6b7280; line-height: 1.6;"> | ||
| <code style="background: #f3f4f6; padding: 2px 6px; border-radius: 4px; font-size: 13px;">npm install @zpress/kit</code> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <!-- Step 2 --> | ||
| <div style="display: flex; gap: 16px;"> | ||
| <div style="display: flex; flex-direction: column; align-items: center; flex-shrink: 0;"> | ||
| <span style="display: flex; align-items: center; justify-content: center; width: 24px; height: 24px; border-radius: 50%; background: #6466f1; color: #fff; font-size: 12px; font-weight: 600;">2</span> | ||
| <div style="width: 2px; flex: 1; background: #e5e7eb; min-height: 16px;"></div> | ||
| </div> | ||
| <div style="padding-bottom: 24px; flex: 1;"> | ||
| <h3 style="margin: 0; font-size: 16px; font-weight: 600; line-height: 24px;">Configure your site</h3> | ||
| <div style="margin-top: 8px; font-size: 14px; color: #6b7280; line-height: 1.6;"> | ||
| Create a <code style="background: #f3f4f6; padding: 2px 6px; border-radius: 4px; font-size: 13px;">zpress.config.ts</code> file in your project root. | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <!-- Step 3 --> | ||
| <div style="display: flex; gap: 16px;"> | ||
| <div style="display: flex; flex-direction: column; align-items: center; flex-shrink: 0;"> | ||
| <span style="display: flex; align-items: center; justify-content: center; width: 24px; height: 24px; border-radius: 50%; background: #6466f1; color: #fff; font-size: 12px; font-weight: 600;">3</span> | ||
| <div style="width: 2px; flex: 1; background: #e5e7eb; visibility: hidden; min-height: 16px;"></div> | ||
| </div> | ||
| <div style="flex: 1;"> | ||
| <h3 style="margin: 0; font-size: 16px; font-weight: 600; line-height: 24px;">Start writing docs</h3> | ||
| <div style="margin-top: 8px; font-size: 14px; color: #6b7280; line-height: 1.6;"> | ||
| Add <code style="background: #f3f4f6; padding: 2px 6px; border-radius: 4px; font-size: 13px;">.mdx</code> files and run the dev server. | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="section" style="margin-top: 32px;"> | ||
| <h3>ParamField</h3> | ||
| <p>Structured parameter documentation with type badges, required/optional indicators, and nesting.</p> | ||
|
|
||
| <div class="mockup"> | ||
| <div class="mockup-header">ParamField — API Parameter Docs</div> | ||
| <div class="mockup-body" style="padding: 24px; font-family: -apple-system, system-ui, sans-serif; background: #fff;"> | ||
| <!-- Field 1: name --> | ||
| <div style="border-left: 2px solid #e5e7eb; padding: 12px 0 12px 16px; margin: 8px 0;"> | ||
| <div style="display: flex; align-items: center; gap: 8px; margin-bottom: 4px;"> | ||
| <span style="font-family: ui-monospace, monospace; font-weight: 600; font-size: 14px; color: #111;">name</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-family: ui-monospace, monospace; font-size: 11px; font-weight: 500; color: #6b7280; background: #f3f4f6;">string</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-size: 11px; font-weight: 600; text-transform: uppercase; letter-spacing: 0.02em; background: rgba(220,38,38,0.12); color: #dc2626;">REQUIRED</span> | ||
| </div> | ||
| <div style="font-size: 14px; color: #6b7280; line-height: 1.6;">The display name of the user.</div> | ||
| </div> | ||
| <!-- Field 2: email --> | ||
| <div style="border-left: 2px solid #e5e7eb; padding: 12px 0 12px 16px; margin: 8px 0; border-top: 1px solid #f0f0f0;"> | ||
| <div style="display: flex; align-items: center; gap: 8px; margin-bottom: 4px;"> | ||
| <span style="font-family: ui-monospace, monospace; font-weight: 600; font-size: 14px; color: #111;">email</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-family: ui-monospace, monospace; font-size: 11px; font-weight: 500; color: #6b7280; background: #f3f4f6;">string</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-size: 11px; font-weight: 600; text-transform: uppercase; color: #9ca3af;">OPTIONAL</span> | ||
| </div> | ||
| <div style="font-size: 14px; color: #6b7280; line-height: 1.6;">Contact email address.</div> | ||
| </div> | ||
| <!-- Field 3: role with default --> | ||
| <div style="border-left: 2px solid #e5e7eb; padding: 12px 0 12px 16px; margin: 8px 0; border-top: 1px solid #f0f0f0;"> | ||
| <div style="display: flex; align-items: center; gap: 8px; margin-bottom: 4px;"> | ||
| <span style="font-family: ui-monospace, monospace; font-weight: 600; font-size: 14px; color: #111;">role</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-family: ui-monospace, monospace; font-size: 11px; font-weight: 500; color: #6b7280; background: #f3f4f6;">string</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-size: 11px; font-weight: 600; text-transform: uppercase; color: #9ca3af;">OPTIONAL</span> | ||
| </div> | ||
| <div style="font-size: 12px; color: #9ca3af; margin-bottom: 4px;">Default: <code style="font-family: ui-monospace, monospace; font-size: 11px; padding: 1px 4px; border-radius: 4px; background: #f3f4f6; color: #6b7280;">"viewer"</code></div> | ||
| <div style="font-size: 14px; color: #6b7280; line-height: 1.6;">The user's role. Controls access permissions.</div> | ||
| </div> | ||
| <!-- Field 4: nested options --> | ||
| <div style="border-left: 2px solid #e5e7eb; padding: 12px 0 12px 16px; margin: 8px 0; border-top: 1px solid #f0f0f0;"> | ||
| <div style="display: flex; align-items: center; gap: 8px; margin-bottom: 4px;"> | ||
| <span style="font-family: ui-monospace, monospace; font-weight: 600; font-size: 14px; color: #111;">options</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-family: ui-monospace, monospace; font-size: 11px; font-weight: 500; color: #6b7280; background: #f3f4f6;">object</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-size: 11px; font-weight: 600; text-transform: uppercase; color: #9ca3af;">OPTIONAL</span> | ||
| </div> | ||
| <div style="font-size: 14px; color: #6b7280; line-height: 1.6;">Additional configuration options.</div> | ||
| <!-- Nested field --> | ||
| <div style="border-left: 2px solid #e5e7eb; padding: 12px 0 12px 16px; margin: 8px 0 0 8px;"> | ||
| <div style="display: flex; align-items: center; gap: 8px; margin-bottom: 4px;"> | ||
| <span style="font-family: ui-monospace, monospace; font-weight: 600; font-size: 14px; color: #111;">verbose</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-family: ui-monospace, monospace; font-size: 11px; font-weight: 500; color: #6b7280; background: #f3f4f6;">boolean</span> | ||
| <span style="padding: 1px 8px; border-radius: 9999px; font-size: 11px; font-weight: 600; text-transform: uppercase; color: #9ca3af;">OPTIONAL</span> | ||
| </div> | ||
| <div style="font-size: 12px; color: #9ca3af; margin-bottom: 4px;">Default: <code style="font-family: ui-monospace, monospace; font-size: 11px; padding: 1px 4px; border-radius: 4px; background: #f3f4f6; color: #6b7280;">false</code></div> | ||
| <div style="font-size: 14px; color: #6b7280; line-height: 1.6;">Enable verbose logging output.</div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Make this a complete HTML document to satisfy lint and preview portability.
component-preview.html is currently fragment-only and will continue to fail HTML linting for missing doctype/root structure.
Suggested structure
+<!DOCTYPE html>
+<html lang="en">
+<head>
+ <meta charset="utf-8">
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+ <title>Component Preview</title>
+</head>
+<body>
<h2>New Components Preview</h2>
...
</div>
+</body>
+</html>🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superpowers/brainstorm/83668-1775853195/content/component-preview.html
around lines 1 - 110, The file is an HTML fragment (starts with <h2>New
Components Preview</h2>) and fails linting; wrap the fragment in a complete HTML
document by adding a doctype and html root, a head with meta charset and
viewport and a <title>, and move the existing fragment into a <body> (preserve
existing classes like "mockup-header"/"mockup-body" and sections such as the
Steps/ParamField content). Ensure proper closing tags for html and body so
component-preview.html passes lint and preview portability.
Comment on lines
+161
to
+163
| function toggleExpandable(el) { | ||
| el.closest('.zp-field-group--expandable').classList.toggle('is-open'); | ||
| } |
Contributor
There was a problem hiding this comment.
Expose expandable state to assistive technologies.
The disclosure UI toggles only CSS class state. Add aria-expanded and aria-controls on trigger buttons and keep aria-expanded synced in toggleExpandable so screen readers can understand collapsed/open state.
Proposed fix
function toggleExpandable(el) {
- el.closest('.zp-field-group--expandable').classList.toggle('is-open');
+ const group = el.closest('.zp-field-group--expandable');
+ const isOpen = group.classList.toggle('is-open');
+ el.setAttribute('aria-expanded', String(isOpen));
}- <button type="button" class="zp-field-group__trigger" onclick="toggleExpandable(this)">
+ <button
+ type="button"
+ class="zp-field-group__trigger"
+ aria-expanded="true"
+ aria-controls="metadata-properties-panel"
+ onclick="toggleExpandable(this)">
...
- <div class="zp-field-group__panel">
+ <div id="metadata-properties-panel" class="zp-field-group__panel">- <button type="button" class="zp-field-group__trigger" onclick="toggleExpandable(this)">
+ <button
+ type="button"
+ class="zp-field-group__trigger"
+ aria-expanded="false"
+ aria-controls="users-array-items-panel"
+ onclick="toggleExpandable(this)">
...
- <div class="zp-field-group__panel">
+ <div id="users-array-items-panel" class="zp-field-group__panel">Also applies to: 214-219, 295-300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superpowers/brainstorm/83668-1775853195/content/field-design.html around
lines 161 - 163, The disclosure currently only toggles the CSS class in function
toggleExpandable; update it to also set aria-expanded on the trigger and ensure
aria-controls exists on the trigger pointing to the expandable region: inside
toggleExpandable (and the other identical handlers), find the trigger element
(the clicked button passed as el or its closest trigger) and update
trigger.setAttribute('aria-expanded', isOpen) when toggling, and ensure the
expandable content has an id (generate a stable unique id if missing) and set
trigger.setAttribute('aria-controls', contentId); keep the existing class toggle
logic but keep aria-expanded in sync on every open/close.
Comment on lines
+214
to
+215
| <button class="zp-field-group__trigger" onclick="toggleExpandable(this)"> | ||
| <svg class="zp-field-group__chevron" viewBox="0 0 24 24" fill="none" stroke="currentColor" stroke-width="2"><path d="M9 18l6-6-6-6"/></svg> |
Contributor
There was a problem hiding this comment.
Add explicit button types for expandable triggers.
Line 214 and Line 295 use <button> without type, which defaults to submit in form contexts and can cause unintended submits.
Proposed fix
- <button class="zp-field-group__trigger" onclick="toggleExpandable(this)">
+ <button type="button" class="zp-field-group__trigger" onclick="toggleExpandable(this)">
...
- <button class="zp-field-group__trigger" onclick="toggleExpandable(this)">
+ <button type="button" class="zp-field-group__trigger" onclick="toggleExpandable(this)">Also applies to: 295-296
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 214-214: The type attribute must be present on elements.
(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superpowers/brainstorm/83668-1775853195/content/field-design.html around
lines 214 - 215, The expandable trigger buttons (the element with class
zp-field-group__trigger that calls toggleExpandable(this) and the other similar
trigger at lines referenced) lack an explicit type, which defaults to submit in
forms; update both <button> elements that invoke toggleExpandable to include
type="button" so they do not submit enclosing forms accidentally—ensure the
change is applied to the button using class zp-field-group__trigger and any
other matching trigger button in the same component.
| <span class="zp-field__badge zp-field__badge--optional">optional</span> | ||
| </div> | ||
| <div class="zp-field__body"> | ||
| Contact email. For deeply nested config, see <a href="#" style="color: var(--zp-c-brand-1);">UserConfig</a>. |
Contributor
There was a problem hiding this comment.
Avoid placeholder links that navigate to page top.
Line 315 uses href="#", which causes a jump and noisy history behavior when clicked. Use a real target URL or a non-link element if navigation is not intended.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superpowers/brainstorm/83668-1775853195/content/field-design.html at line
315, The anchor using href="#" for the "UserConfig" link causes page jump and
bad history; replace it with a real URL to the UserConfig documentation if
navigation is intended, or convert the element to a non-navigating control
(e.g., a <span> or <button> with appropriate role/aria attributes and an onclick
handler) if it should open a modal/tool-tip or trigger JS, and remove the
href="#" to prevent history navigation; update the element referenced as
"UserConfig" accordingly and ensure accessibility attributes are present when
changing to a non-link control.
Comment on lines
+1
to
+3
| <div style="display:flex;align-items:center;justify-content:center;min-height:60vh"> | ||
| <p class="subtitle">Building Steps and ParamField components...</p> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Add a valid HTML document wrapper (or mark this as a fragment).
This file is a raw fragment, but it is committed as .html and currently violates doctype-first. If this is meant to be standalone preview content, wrap it with <!DOCTYPE html><html><head>.... If it is intentionally a partial, exclude this path from HTMLHint.
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 1-1: Doctype must be declared before any non-comment content.
(doctype-first)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.superpowers/brainstorm/83668-1775853195/content/waiting.html around lines 1
- 3, This fragment currently lacks a document wrapper and violates
doctype-first; either wrap the existing root <div> (the element with
style="display:flex;..." containing the <p class="subtitle">Building Steps and
ParamField components...</p>) in a valid HTML document (prepend <!DOCTYPE html>,
add <html><head> with at least a <meta charset="utf-8"> and optional <title>,
then place the existing <div> inside <body>) or, if it is intentionally a
partial, keep it as-is but mark it as a fragment by excluding this file from
HTMLHint (or move to a fragment-only directory) so the linter doesn't enforce
doctype-first.
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
… promotion Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…section handling Co-Authored-By: Claude <noreply@anthropic.com>
Root sections are expanded into their visible children before flattening, and the root parent directory is filtered from the output so no parent group _meta.json is emitted. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
zrosenbauer
force-pushed
the
feat/root-section
branch
from
60376f2 to
76a6075
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/config/src/schema.ts (1)
99-121:⚠️ Potential issue | 🟠 MajorValidate
rootonly where the sync pipeline actually supports it.
entrySchemanow acceptsrooton every recursiveSection, but the implementation only givesrootsemantics to top-level grouped sections. Nesteditems[].rootvalues are never promoted by the top-level sidebar/meta builders, and leaf sections do not carryrootthrough the leaf resolution paths. That means unsupported shapes now validate successfully and then get silently ignored. Either rejectrootoutside supported section shapes at the config boundary, or make the downstream collectors handle it recursively.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/config/src/schema.ts` around lines 99 - 121, entrySchema currently allows the `root` flag on any nested Section (via entrySchema and items recursion) but the sync/collector logic (sidebar/meta builders and leaf resolution paths) only honors `root` on top-level grouped sections, causing unsupported nested `root` values to be silently ignored; fix by narrowing validation so `root` is only permitted on the section shapes the pipeline handles (e.g., top-level grouped Section) or by updating the collectors to propagate `root` recursively—specifically, restrict `root` in entrySchema (and Section type) for nested items, or update the sidebar/meta builders and leaf resolution codepaths to detect and honor items[].root and leaf section root flags consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/sync/sidebar/meta.ts`:
- Around line 128-142: The current filter removes entire directory groups when
their dirPath is a root parent, which drops other siblings; in
buildMetaDirectories, after computing placements via flattenToPlacements and
grouping with groupPlacementsByDir, iterate each dir's placements and remove
only the specific placement that represents the root parent wrapper (identify it
by matching the placement's link/path to entries that are root && link — use the
rootParentDirs set), then keep the dir if any placements remain; i.e., instead
of filtering groupPlacementsByDir(placements) by dir.dirPath against
rootParentDirs, filter each dir.placements to exclude the wrapper placement and
discard dirs that become empty.
- Around line 83-100: The current code in meta.ts always emits promoted root
children as { type: 'dir' } which is wrong for leaf pages; update the flatMap
that processes entry.items (the Section.items promotion for entries with
entry.root) to inspect each child: if the child has its own items (i.e., is a
subsection) keep emitting { type: 'dir', name: resolveDirName(child), label:
child.title }, but if the child is a leaf page emit { type: 'file', name:
resolveDirName(child), label: child.title } (and still skip when resolveDirName
returns null); use the presence/absence of child.items to distinguish file vs
dir rather than always 'dir'.
In `@packages/core/src/sync/sidebar/nav.test.ts`:
- Around line 55-59: The test currently guards child assertions with "if
(refItem && 'items' in refItem)" which allows a false-positive when Reference
exists but has no items; update the assertions to explicitly require items
before mapping: after ensuring refItem is defined
(expect(refItem).toBeDefined()), add assertions that refItem.items is defined
and is an array (e.g., expect(refItem.items).toBeDefined() and
expect(Array.isArray(refItem.items)).toBe(true)) and only then extract
childTexts from refItem.items to assert it equals ['API', 'CLI']; reference the
refItem variable and its items property in nav.test.ts.
---
Outside diff comments:
In `@packages/config/src/schema.ts`:
- Around line 99-121: entrySchema currently allows the `root` flag on any nested
Section (via entrySchema and items recursion) but the sync/collector logic
(sidebar/meta builders and leaf resolution paths) only honors `root` on
top-level grouped sections, causing unsupported nested `root` values to be
silently ignored; fix by narrowing validation so `root` is only permitted on the
section shapes the pipeline handles (e.g., top-level grouped Section) or by
updating the collectors to propagate `root` recursively—specifically, restrict
`root` in entrySchema (and Section type) for nested items, or update the
sidebar/meta builders and leaf resolution codepaths to detect and honor
items[].root and leaf section root flags consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5a360828-551f-4a25-aa7b-d90a9ffb05ad
📒 Files selected for processing (10)
packages/config/schemas/schema.jsonpackages/config/src/schema.tspackages/config/src/types.tspackages/core/src/sync/index.tspackages/core/src/sync/resolve/index.tspackages/core/src/sync/sidebar/index.tspackages/core/src/sync/sidebar/meta.test.tspackages/core/src/sync/sidebar/meta.tspackages/core/src/sync/sidebar/nav.test.tspackages/core/src/sync/types.ts
Comment on lines
83
to
+100
| .flatMap((entry) => { | ||
| if (entry.root && entry.items) { | ||
| return entry.items | ||
| .filter((child) => !child.hidden) | ||
| .flatMap((child) => { | ||
| const name = resolveDirName(child) | ||
| if (name === null) { | ||
| return [] | ||
| } | ||
| return [ | ||
| { | ||
| type: 'dir' as const, | ||
| name, | ||
| label: child.title, | ||
| }, | ||
| ] | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
Don't emit every promoted root child as a directory.
The root branch always returns { type: 'dir' }, but Section.items can contain leaf pages as well as subsections. With a root section that directly contains a page child, the generated root _meta.json will point Rspress at a directory that does not exist. Emit file vs dir based on the promoted child's shape, or validate that root sections may only contain child sections.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/sync/sidebar/meta.ts` around lines 83 - 100, The current
code in meta.ts always emits promoted root children as { type: 'dir' } which is
wrong for leaf pages; update the flatMap that processes entry.items (the
Section.items promotion for entries with entry.root) to inspect each child: if
the child has its own items (i.e., is a subsection) keep emitting { type: 'dir',
name: resolveDirName(child), label: child.title }, but if the child is a leaf
page emit { type: 'file', name: resolveDirName(child), label: child.title } (and
still skip when resolveDirName returns null); use the presence/absence of
child.items to distinguish file vs dir rather than always 'dir'.
Comment on lines
128
to
+142
| export function buildMetaDirectories(entries: readonly ResolvedEntry[]): readonly MetaDirectory[] { | ||
| const { placements } = flattenToPlacements(entries, 0) | ||
| return groupPlacementsByDir(placements) | ||
| const rootParentDirs = new Set( | ||
| entries | ||
| .filter((entry) => entry.root && entry.link) | ||
| .map((entry) => stripLeadingSlash(entry.link ?? '')) | ||
| .filter(Boolean) | ||
| ) | ||
| const expanded = entries.flatMap((entry) => { | ||
| if (entry.root && entry.items) { | ||
| return entry.items.filter((child) => !child.hidden) | ||
| } | ||
| return [entry] | ||
| }) | ||
| const { placements } = flattenToPlacements(expanded, 0) | ||
| return groupPlacementsByDir(placements).filter((dir) => !rootParentDirs.has(dir.dirPath)) |
Contributor
There was a problem hiding this comment.
Filtering the whole parent directory drops mixed siblings.
groupPlacementsByDir(placements).filter((dir) => !rootParentDirs.has(dir.dirPath)) removes every placement under the root parent's filesystem directory. If a root section at /references coexists with another non-root entry that also lives under references/*, that sibling disappears from _meta.json entirely. The filter needs to suppress only the wrapper placement for the root parent, not the entire directory bucket.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/sync/sidebar/meta.ts` around lines 128 - 142, The current
filter removes entire directory groups when their dirPath is a root parent,
which drops other siblings; in buildMetaDirectories, after computing placements
via flattenToPlacements and grouping with groupPlacementsByDir, iterate each
dir's placements and remove only the specific placement that represents the root
parent wrapper (identify it by matching the placement's link/path to entries
that are root && link — use the rootParentDirs set), then keep the dir if any
placements remain; i.e., instead of filtering groupPlacementsByDir(placements)
by dir.dirPath against rootParentDirs, filter each dir.placements to exclude the
wrapper placement and discard dirs that become empty.
Comment on lines
+55
to
+59
| expect(refItem).toBeDefined() | ||
| if (refItem && 'items' in refItem) { | ||
| const childTexts = (refItem.items as readonly { readonly text: string }[]).map((c) => c.text) | ||
| expect(childTexts).toEqual(['API', 'CLI']) | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent false-positive pass when dropdown items are missing.
On Line 56, child assertions only run conditionally. If Reference exists but has no items, this test still passes.
Suggested fix
const nav = generateNav(autoConfig, entries)
const refItem = nav.find((item) => item.text === 'Reference')
expect(refItem).toBeDefined()
- if (refItem && 'items' in refItem) {
- const childTexts = (refItem.items as readonly { readonly text: string }[]).map((c) => c.text)
+ expect(refItem && 'items' in refItem).toBe(true)
+ if (refItem && 'items' in refItem) {
+ const childTexts = refItem.items.map((c) => c.text)
expect(childTexts).toEqual(['API', 'CLI'])
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| expect(refItem).toBeDefined() | |
| if (refItem && 'items' in refItem) { | |
| const childTexts = (refItem.items as readonly { readonly text: string }[]).map((c) => c.text) | |
| expect(childTexts).toEqual(['API', 'CLI']) | |
| } | |
| const nav = generateNav(autoConfig, entries) | |
| const refItem = nav.find((item) => item.text === 'Reference') | |
| expect(refItem).toBeDefined() | |
| expect(refItem && 'items' in refItem).toBe(true) | |
| if (refItem && 'items' in refItem) { | |
| const childTexts = refItem.items.map((c) => c.text) | |
| expect(childTexts).toEqual(['API', 'CLI']) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/sync/sidebar/nav.test.ts` around lines 55 - 59, The test
currently guards child assertions with "if (refItem && 'items' in refItem)"
which allows a false-positive when Reference exists but has no items; update the
assertions to explicitly require items before mapping: after ensuring refItem is
defined (expect(refItem).toBeDefined()), add assertions that refItem.items is
defined and is an array (e.g., expect(refItem.items).toBeDefined() and
expect(Array.isArray(refItem.items)).toBe(true)) and only then extract
childTexts from refItem.items to assert it equals ['API', 'CLI']; reference the
refItem variable and its items property in nav.test.ts.
…I references Demonstrates `root: true` — "Reference" section promotes "API" and "CLI" as top-level sidebar items without a wrapping parent group. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
root: truesection property that promotes child sections to top-level sidebar items while hiding the parent title from the sidebar hierarchyCloses #93
Changes
Config (
packages/config):root?: booleanadded toSectioninterface and Zod schemaCore (
packages/core):rootpropagated toResolvedEntryand resolverbuildRootMetapromotes root children to top-level_meta.jsonitemsbuildMetaDirectoriesexpands root sections so no parent group directory is emittedcollectStandaloneScopePathsincludes root sections inscopes.jsongenerateNavandresolveChildrentreat root sections like standalone (dropdown nav)Tests: 9 new tests (6 meta, 3 nav) — all 158 passing
Test plan
buildRootMetapromotes root children, excludes parent, filters hiddenbuildMetaDirectoriesomits parent directory group, emits child subdirectories, handles mixed root/non-rootgenerateNavexcludes root from non-standalone limit, produces dropdown children